Skip to content

Fix workspace-scoped Codex skill discovery - #3059

Closed
Quicksaver wants to merge 130 commits into
pingdotgg:mainfrom
Quicksaver:fix/codex-skills
Closed

Fix workspace-scoped Codex skill discovery#3059
Quicksaver wants to merge 130 commits into
pingdotgg:mainfrom
Quicksaver:fix/codex-skills

Conversation

@Quicksaver

@Quicksaver Quicksaver commented Jun 12, 2026

Copy link
Copy Markdown

Summary

Fix Codex repo-local skill discovery by resolving skills for the active project or worktree cwd instead of relying on the provider-wide status snapshot.

The implementation adds a workspace-scoped provider-skills RPC and a shared web/mobile resolution policy so composer suggestions and rendered skill chips refresh for the correct workspace without leaking stale skills across workspace, environment, or provider changes.

What Changed

  • Add server.listProviderSkills contracts, a focused provider-skills RPC handler, WebSocket and local API routing, and environment-query wiring keyed by environment, provider instance, and cwd.
  • Add ProviderSkillsLister to return snapshot skills for non-Codex or disabled Codex providers and run Codex skills/list in the validated workspace cwd for enabled Codex providers.
  • Prepare the configured Codex home and forward configured launch arguments before each probe, and return bounded structured errors for provider lookup, settings, cwd, home-preparation, unauthenticated accounts, timeout, and app-server failures while projecting both Error instances and plain objects through one bounded diagnostic path.
  • Coalesce requests by provider and normalized workspace root with a short TTL cache for successful lookups only, keep failed probes immediately retryable, bound cache capacity and concurrent probes, tolerate parallel requests racing to create the same correct shadow-home symlink, and terminate short-lived Codex app-server processes when requests finish or time out.
  • Run Codex provider status probes with the configured server cwd instead of the backend process cwd.
  • Share workspace-skill target preparation, query-state resolution, snapshot retention, fallback selection, and error formatting across web and mobile adapters: preserve settled skills during a same-workspace refresh, clear them while switching targets or after a failed refresh, and fall back to provider snapshot skills after empty or failed lookups while keeping errors visible, without refreshing workspace probes when only client-side fallback skills change.
  • Share settings-adjusted provider-instance selection across the web composer, web timeline, and mobile workspace-skill adapters, including enabled state, availability, continuation groups, default-first fallback ordering, session-instance precedence, semantic driver/instance separation, stale-routing fallback, canonical-default recovery through remaining same-driver instances, and registry-loading behavior without custom-slug guesses. Timeline decoration keeps the active session and persisted thread model ahead of unsaved next-turn draft selection so changing the model picker cannot retarget already-sent skill references.
  • Align mobile workspace-skill lookup with environment connectivity and lazy activation: disconnected clients retain only verified same-workspace skills or provider fallback with a reconnect error, inactive same-workspace lookups preserve their verified snapshot, and pending workspace switches clear snapshots from other targets.
  • Keep mobile thread-detail lookup lazy: empty thread opens do not start workspace RPCs, while the composer $ menu, complete draft skill tokens, and sent user skill references activate the shared lookup for composer and feed metadata.
  • Load composer workspace skills only when needed for the $ menu or existing complete $skill references, treating whitespace, punctuation, and end-of-input as shared completion boundaries for both lookup and web chip rendering, and load timeline skills only when a sent user prompt contains a complete skill reference; mobile promotes active skill triggers before the menu paints, while web and mobile new-worktree drafts deliberately use snapshot fallback until their cwd exists and never probe the base checkout for repo-local skills.
  • Reuse workspace-scoped skills for web and mobile composer suggestions, inline editor chips, pending answers, and submitted user-message skill chips, with shared token-boundary handling across web and mobile rendering.
  • Add focused coverage for the provider-skills handler and WebSocket wire serialization, workspace validation, Codex app-server cwd and lifecycle behavior, timeout and request coalescing, query-key isolation, stale-state prevention, lazy skill references, fallback/error rendering, search ranking, and timeline chips, with shared provider-snapshot fixtures across handler and router tests.

Why

Fixes #3040.

Codex discovers repo-local skills relative to the cwd supplied to skills/list, but a provider status snapshot is global to the provider instance. A workspace-scoped query is therefore required to show the correct skills for each project and worktree, especially while switching between workspaces or refreshing under failure.

Validation

  • Focused server, client-runtime, web, mobile, and shared-package suites passed for the provider-skills RPC and lister, Codex probe lifecycle and diagnostics, workspace-state transitions, provider-instance selection, lazy lookup activation, inline skill-token parsing, and skill-chip rendering.
  • Scoped formatting and lint checks passed for the affected TypeScript files. Client-runtime and mobile package typechecks passed; pnpm exec vp check completed with 0 errors and 9 existing React warnings, pnpm exec vp run typecheck completed with all 15 tasks passing, and pnpm exec vp run lint:mobile passed.
  • Integrated web verification on web 5735 and server 13775 showed all 4 repo-local skills for the selected codex-skills checkout, retained them across lazy-menu reopen, rendered complete end-of-input and punctuation-terminated skill references correctly, decorated sent timeline references with the session-bound provider, and avoided probing a future-worktree draft until its cwd existed.
  • Integrated iOS verification against the isolated backend showed the skill menu transition from loading to all 4 repo-local skills, converted a complete $ios-debugger-agent reference into a native inline skill attachment, and produced a non-empty live simulator stream.
  • git diff --check upstream/main...HEAD passed after merging the current upstream main.

Proof

  • No external proof artifacts; validation is covered by the automated checks above.

Note

Medium Risk
Touches Codex app-server probes, WebSocket RPC, and cross-platform skill state; bounded caching and lazy loading limit blast radius, but cwd/home preparation and auth failures are user-visible in composer/timeline.

Overview
Fixes repo-local Codex skills showing up in the wrong workspace by listing skills against the active project/worktree cwd instead of the global provider status snapshot.

Server: Adds server.listProviderSkills with a shared ProviderSkillsLister that validates cwd, prepares Codex home, runs Codex skills/list with timeouts and process cleanup, returns snapshot skills for non-Codex/disabled Codex, and emits bounded structured errors. Successful lookups are cached briefly with request coalescing and bounded concurrency; shadow-home symlink creation tolerates concurrent winners. checkCodexProviderStatus now uses the configured server cwd, not the backend process cwd.

Clients: Web and mobile use useProviderWorkspaceSkills and shared @t3tools/client-runtime policy (snapshot retention, fallback on empty/error, disconnect handling). Workspace RPCs run lazily—when the $ menu is open or a complete $skill token appears (whitespace, punctuation, or end-of-input)—including timeline decoration for sent user messages. @t3tools/shared/skillInlineTokens tightens boundaries so paths like $HOME are not skill chips. Future worktree drafts keep snapshot fallback until a real cwd exists.

UI: Composer and timeline skill pickers/chips use workspace skills; skill lookup errors stay visible alongside fallback skills. Provider instance selection is centralized so timeline decoration follows session/thread model, not unsaved draft picker changes.

Reviewed by Cursor Bugbot for commit c79ac8d. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Fix workspace-scoped Codex skill discovery by adding a live skills RPC endpoint and UI integration

  • Adds listCodexProviderSkills to spawn a Codex app-server scoped to a supplied cwd and return parsed skills; unauthenticated state surfaces as a typed CodexProviderSkillsUnauthenticatedError.
  • Exposes a new server.listProviderSkills WebSocket RPC method (with AuthOrchestrationReadScope enforcement) backed by a bounded, concurrency-limited cache that coalesces in-flight requests and caches only successes.
  • Adds useProviderWorkspaceSkills hooks on web and mobile that issue the new RPC, gate loading behind inline skill token detection and connection availability, and fall back to provider snapshot skills.
  • Moves skill token parsing to a shared @t3tools/shared/skillInlineTokens package with stricter boundary rules: tokens must be followed by end-of-string, whitespace, or sentence punctuation, so $HOME/.codex and $value; are no longer matched.
  • Centralizes provider instance selection into resolveProviderInstanceSelection (shared via @t3tools/client-runtime/state/provider-workspace-skills) and wires it through ChatView, ChatComposer, ThreadComposer, and ThreadDetailScreen.
  • Fixes a race condition in CodexHomeLayout.ensureSymlink where concurrent materialization of the same shadow home would fail; it now succeeds if the existing symlink already points to the target.
  • Risk: checkCodexProviderStatus now requires a caller-supplied cwd instead of using process.cwd(), which is a breaking change for any direct callers outside this PR.

Macroscope summarized c79ac8d.

- Propagate cwd through provider status probes
- Add server RPC for workspace skill discovery
- Load workspace skills in the composer UI
- Clarify bogus skill as a durable discovery test fixture

- Stabilize composer fallback skill array identity
- Skip Codex skill spawning for disabled instances

- Move bogus skill fixture out of workspace discovery
- Refresh workspace skill cache on provider and connection changes

- Validate Codex skill cwd before spawning the app server

- Cover server.listProviderSkills RPC branches
- Track the active workspace key in provider skill state

- Reset pending skills when switching workspace targets
@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e5d48272-1b00-47bc-8098-8e15d2391265

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Jun 12, 2026
Comment thread apps/web/src/lib/providerWorkspaceSkillsState.ts Outdated
Comment thread apps/web/src/components/chat/ChatComposer.tsx
Comment thread apps/server/src/ws.ts Outdated
@macroscopeapp

macroscopeapp Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs human review

This PR introduces a new workspace-scoped skill discovery feature with a new RPC endpoint, significant state management changes across server/web/mobile, and runtime behavior modifications affecting how skills are resolved and displayed. The scope and new capability introduction warrant human review.

You can customize Macroscope's approvability policy. Learn more.

- Keep pending skill lookups scoped to the active workspace key
- Surface Codex skill-list timeout errors in the composer
- Add regression coverage for stale pending skills
@github-actions github-actions Bot added size:XL 500-999 changed lines (additions + deletions). and removed size:L 100-499 changed lines (additions + deletions). labels Jun 12, 2026
# Conflicts:
#	apps/server/src/provider/Layers/CursorProvider.ts
#	apps/web/src/components/chat/ChatComposer.tsx
@github-actions github-actions Bot added size:L 100-499 changed lines (additions + deletions). and removed size:XL 500-999 changed lines (additions + deletions). labels Jun 19, 2026
Quicksaver and others added 2 commits June 19, 2026 14:07
# Conflicts:
#	apps/web/src/environments/runtime/connection.test.ts
#	apps/web/src/environments/runtime/service.savedEnvironments.test.ts
#	apps/web/src/environments/runtime/service.threadSubscriptions.test.ts
#	apps/web/src/localApi.ts
#	packages/client-runtime/src/wsRpcClient.ts
Co-authored-by: codex <codex@users.noreply.github.com>
Comment thread apps/web/src/lib/providerWorkspaceSkillsState.ts Outdated
- Keep prior workspace skills visible while refreshes are pending
- Add tests for loaded, pending, workspace-switch, and empty states
- Preserve and clear cached workspace skills by query state

- Cover pending data, empty data, and workspace switches
@github-actions github-actions Bot added size:XL 500-999 changed lines (additions + deletions). and removed size:L 100-499 changed lines (additions + deletions). labels Jun 19, 2026
- Match session driver names only against driver semantics
- Resolve session, thread, and selected routing values only as instance IDs
- Fall through stale instances without guessing during registry loading
Comment thread packages/client-runtime/src/state/providerWorkspaceSkills.ts
T3 Verification added 7 commits July 28, 2026 19:47
- Map removed canonical default instance IDs through remaining driver entries
- Preserve exact instance precedence and avoid guessing custom slugs
- Cover shared and web started-thread lock recovery
- Assert bounded diagnostics before and after RPC serialization
- Share provider snapshots across focused and router tests
- Reuse the lister function without an extra request span
Comment thread apps/server/src/provider/Layers/CodexProvider.ts
T3 Verification added 2 commits July 29, 2026 10:29
- Fail workspace skill probes when Codex requires authentication
- Cover structured provider-skill errors with the mock app server
Comment thread apps/web/src/components/ChatView.tsx
T3 Verification added 7 commits July 29, 2026 11:57
- Prioritize session and persisted thread instances for timeline lookup
- Cover draft provider changes with focused web regression tests
# Conflicts:
#	apps/server/src/ws.ts
#	apps/web/src/components/ChatView.logic.ts
#	apps/web/src/localApi.ts
#	packages/client-runtime/src/state/server.ts
#	packages/contracts/src/ipc.ts
Comment thread apps/mobile/src/features/threads/thread-provider-skills.ts
Comment thread apps/mobile/src/features/threads/ThreadComposer.tsx

@macroscopeapp macroscopeapp Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One Effect service convention issue found in the new server module. Everything else in the changed Effect code (structured ServerProviderSkillsListError attributes, cause preservation, namespace imports of effect/* subpaths, dependency acquisition via yield* in makeProviderSkillsLister) looks consistent with the conventions.

Posted via Macroscope — Effect Service Conventions

Comment thread apps/server/src/provider/ProviderSkillsLister.ts Outdated
- Load complete draft skill tokens at shared text boundaries
- Activate mobile skill lookup before the menu paints
- Use namespace imports for local Effect services
Comment thread apps/server/src/provider/ProviderSkillsLister.ts
Comment thread apps/web/src/composer-editor-mentions.ts
T3 Verification added 2 commits July 29, 2026 23:18
- Tolerate concurrent shadow-home symlink creation
- Render completed composer skills at shared boundaries
- Cover parallel preparation and browser token segments

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit c79ac8d. Configure here.

enabled: providerWorkspaceSkillsLookupEnabled,
connectionAvailable: props.connectionStateLabel === "connected",
fallbackSkills: selectedProviderFallbackSkills,
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mobile composer skills ignore draft

Medium Severity

Thread detail drives both the feed and composer from one useProviderWorkspaceSkills call whose provider instance is resolved with the session instance ahead of the draft model. After an active session, changing the next-turn model picker updates the composer UI, but workspace skill lookup and chips still target the session’s Codex instance instead of the draft selection.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit c79ac8d. Configure here.

@juliusmarminge

Copy link
Copy Markdown
Member

Closing in favor of #2829 (orchestration V2).

#2829 deletes the V1 orchestration layer this PR builds on — apps/server/src/orchestration/**, provider/Layers/*Adapter.ts and provider/Services/** are removed and replaced by apps/server/src/orchestration-v2/**, with the IPC surface renamed to ORCHESTRATION_V2_WS_METHODS. The files this PR touches either no longer exist or are rewritten, so it can't be rebased — it would need reimplementing against the V2 adapters.

This is not a judgement on the change itself. Several of these are real gaps we still want fixed; the base just moved out from under them.

Once #2829 merges, please rebase onto main, port the change to the V2 equivalent, and reopen (or open a fresh PR). Ping me and I'll prioritise the review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XXL 1,000+ changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Repo-local Codex skills do not appear in T3 Code $ skill picker

3 participants